Aggregate packages #3689

Rosuavio · 2021-01-25T18:39:38Z

Step towards #3594

Adds a project that builds a package that aggregates together other UI control packages.

PR Type

What kind of change does this PR introduce?

Feature

Refactoring (no functional changes, no api changes)
Build or CI related changes

What is the current behavior?

What is the new behavior?

PR Checklist

Please check if your PR fulfills the following requirements:

Tested code with current supported SDKs
Pull Request has been submitted to the documentation repository instructions. Link:
Sample in sample app has been added / updated (for bug fixes / features)
- Icon has been created (if new sample) following the Thumbnail Style Guide and templates
New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
Tests for the changes have been added (for bug fixes / features) (if applicable)
Header has been added to all new source files (run build/UpdateHeaders.bat)
Contains NO breaking changes

Other information

ghost · 2021-01-25T18:39:42Z

Thanks RosarioPulella for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

Rosuavio · 2021-02-02T22:04:55Z

Microsoft.Toolkit.Uwp.UI.Controls/Microsoft.Toolkit.Uwp.UI.Controls.csproj

+      <PackagingOutputs Remove="@(PackagingOutputs)" Condition="'%(PackagingOutputs.Filename)%(PackagingOutputs.Extension)' == 'Microsoft.Toolkit.Uwp.UI.Controls.Core.pri'" />
+      <PackagingOutputs Remove="@(PackagingOutputs)" Condition="'%(PackagingOutputs.Filename)%(PackagingOutputs.Extension)' == 'Microsoft.Toolkit.Uwp.UI.Controls.Primitives.pri'" />
+      <PackagingOutputs Remove="@(PackagingOutputs)" Condition="'%(PackagingOutputs.Filename)%(PackagingOutputs.Extension)' == 'Microsoft.Toolkit.Uwp.UI.Controls.Media.pri'" />
+      <PackagingOutputs Remove="@(PackagingOutputs)" Condition="'%(PackagingOutputs.Filename)%(PackagingOutputs.Extension)' == 'Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.pri'" />
+      <PackagingOutputs Remove="@(PackagingOutputs)" Condition="'%(PackagingOutputs.Filename)%(PackagingOutputs.Extension)' == 'Microsoft.Toolkit.Uwp.UI.Controls.Markdown.pri'" />
+      <PackagingOutputs Remove="@(PackagingOutputs)" Condition="'%(PackagingOutputs.Filename)%(PackagingOutputs.Extension)' == 'Microsoft.Toolkit.Uwp.UI.Controls.Layout.pri'" />
+      <PackagingOutputs Remove="@(PackagingOutputs)" Condition="$([System.String]::new('%(PackagingOutputs.TargetPath)').StartsWith('Microsoft.Toolkit.Uwp.UI.Controls.Primitives\'))" />
+      <PackagingOutputs Remove="@(PackagingOutputs)" Condition="$([System.String]::new('%(PackagingOutputs.TargetPath)').StartsWith('Microsoft.Toolkit.Uwp.UI.Controls.Core\'))" />
+      <PackagingOutputs Remove="@(PackagingOutputs)" Condition="$([System.String]::new('%(PackagingOutputs.TargetPath)').StartsWith('Microsoft.Toolkit.Uwp.UI.Controls.Media\'))" />
+      <PackagingOutputs Remove="@(PackagingOutputs)" Condition="$([System.String]::new('%(PackagingOutputs.TargetPath)').StartsWith('Microsoft.Toolkit.Uwp.UI.Controls.DataGrid\'))" />
+      <PackagingOutputs Remove="@(PackagingOutputs)" Condition="$([System.String]::new('%(PackagingOutputs.TargetPath)').StartsWith('Microsoft.Toolkit.Uwp.UI.Controls.Markdown\'))" />
+      <PackagingOutputs Remove="@(PackagingOutputs)" Condition="$([System.String]::new('%(PackagingOutputs.TargetPath)').StartsWith('Microsoft.Toolkit.Uwp.UI.Controls.Layout\'))" />


@michael-hawker I don't have a lot of knowledge on how the original "RemoveUnwantedPri" code in other projects but, this code seemed to have fixed issues when including the package in a test project for me, let me know if this code here makes sense.

Also there has to be a better way to write this, I am not that familiar with what syntax is available here. I anyone wants to help me clean this up.

I wonder if this is because we're using ProjectReference,.. @azchohfi could we just use PackageReference here and see if that removes the problem? Or that's going to be an issue, eh? As then we'd have to build it after the others?

But otherwise, yes I think we need these to work-around how the build system works. Ideally these packages don't contain any content, so we may need to look at the final nuget package built in the end.

It would make our life a mess to sync the version. I don't see why we would need these, as the package itself should not contain anything. If adding the <IncludeBuildOutput>false</IncludeBuildOutput> don't fix it, which I think it will, we could always just use a nuspec.

Rosuavio · 2021-02-02T22:13:31Z

I tested this by taking the published "Controls" package into UWP app and using the InfiniteCanvas, testing if other controls can be used just fine would be good.

michael-hawker · 2021-02-09T17:51:47Z

@azchohfi we wouldn't expect there to be a dll here, eh?

azchohfi · 2021-02-09T17:54:53Z

There will still be an auto generated class for assembly version, etc, so it will, by default, generate a DLL. There is a property to exclude it. Let me find it.

azchohfi · 2021-02-09T17:55:10Z

<IncludeBuildOutput>false</IncludeBuildOutput>

Only include depedencies in package.

Rosuavio · 2021-02-09T20:48:25Z

Thanks @azchohfi, from the smoke tests it looks like it worked.

michael-hawker · 2021-02-09T20:55:08Z

Looks like we're clean now!

@RosarioPulella wondering if we can remove the PRI work-arounds now that it's not building the DLL? Want to give that a quick test on your local branch and see?

@azchohfi otherwise I think we're all good, eh?

Rosuavio · 2021-02-10T16:33:27Z

@RosarioPulella wondering if we can remove the PRI work-arounds now that it's not building the DLL? Want to give that a quick test on your local branch and see?

Worked just fine locally. Feel free to test it from the nuget feed once CI finishes.

ghost · 2021-02-11T21:16:55Z

Hello @michael-hawker!

Because this pull request has the auto merge :zap: label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

Microsoft.Toolkit.Uwp.UI.Controls/Microsoft.Toolkit.Uwp.UI.Controls.csproj

michael-hawker · 2021-02-11T23:18:19Z

Looks great, new build works the same. 🎉

Rosuavio added in progress 🚧 DO NOT MERGE ⚠️ labels Jan 25, 2021

ghost requested review from michael-hawker, azchohfi and Kyaa-dost January 25, 2021 18:39

Rosuavio mentioned this pull request Feb 1, 2021

Rename controls to core #3676

Merged

8 tasks

Rosuavio commented Feb 2, 2021

View reviewed changes

Rosuavio added build 🔥 controls 🎛️ for-review 📖 To evaluate and validate the Issues or PR functional testing required 📌 reviewers wanted ✋ and removed DO NOT MERGE ⚠️ in progress 🚧 labels Feb 2, 2021

Rosuavio marked this pull request as ready for review February 2, 2021 22:09

Rosuavio changed the title ~~Aggregate packages~~ [WIP] Aggregate packages Feb 4, 2021

Rosuavio marked this pull request as draft February 4, 2021 17:22

Rosuavio changed the title ~~[WIP] Aggregate packages~~ Aggregate packages Feb 8, 2021

Rosuavio marked this pull request as ready for review February 8, 2021 20:19

michael-hawker added the next preview ✈️ Label for marking what we want to include in the next preview release for developers to try. label Feb 8, 2021

Rosuavio added 5 commits February 9, 2021 11:07

Use csporj to aggrate controls

c689a51

WIP: Try to fix erros when using aggregae package

057f5fa

Controls: remove unwanted pri files.

bb49ce0

Add Controls.Input to meta package

da272cb

Added meta package smoke test

14df8de

Remove UI.Controls project build output from package

2482745

Only include depedencies in package.

Remove custom RemoveUnwantedPri build target, not needed

70928b4

michael-hawker approved these changes Feb 11, 2021

View reviewed changes

michael-hawker added the auto merge ⚡ label Feb 11, 2021

Rosuavio commented Feb 11, 2021

View reviewed changes

Microsoft.Toolkit.Uwp.UI.Controls/Microsoft.Toolkit.Uwp.UI.Controls.csproj Outdated Show resolved Hide resolved

Remove EnableTypeInfoReflection

61c08f2

Rosuavio commented Feb 11, 2021

View reviewed changes

Microsoft.Toolkit.Uwp.UI.Controls/Microsoft.Toolkit.Uwp.UI.Controls.csproj Outdated Show resolved Hide resolved

Remove EnableTypeInfoReflection comment

ae3c914

michael-hawker merged commit c7bdaf2 into CommunityToolkit:dev/split-controls Feb 11, 2021

Rosuavio deleted the aggregate-packages branch February 12, 2021 14:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Aggregate packages #3689

Aggregate packages #3689

Rosuavio commented Jan 25, 2021 •

edited

Loading

ghost commented Jan 25, 2021

Rosuavio Feb 2, 2021

michael-hawker Feb 8, 2021

azchohfi Feb 9, 2021

Rosuavio commented Feb 2, 2021

michael-hawker commented Feb 9, 2021

azchohfi commented Feb 9, 2021

azchohfi commented Feb 9, 2021 •

edited

Loading

Rosuavio commented Feb 9, 2021

michael-hawker commented Feb 9, 2021

Rosuavio commented Feb 10, 2021

ghost commented Feb 11, 2021

michael-hawker commented Feb 11, 2021

Aggregate packages #3689

Aggregate packages #3689

Conversation

Rosuavio commented Jan 25, 2021 • edited Loading

Step towards #3594

PR Type

What is the current behavior?

What is the new behavior?

PR Checklist

Other information

ghost commented Jan 25, 2021

Rosuavio Feb 2, 2021

Choose a reason for hiding this comment

michael-hawker Feb 8, 2021

Choose a reason for hiding this comment

azchohfi Feb 9, 2021

Choose a reason for hiding this comment

Rosuavio commented Feb 2, 2021

michael-hawker commented Feb 9, 2021

azchohfi commented Feb 9, 2021

azchohfi commented Feb 9, 2021 • edited Loading

Rosuavio commented Feb 9, 2021

michael-hawker commented Feb 9, 2021

Rosuavio commented Feb 10, 2021

ghost commented Feb 11, 2021

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

michael-hawker commented Feb 11, 2021

Rosuavio commented Jan 25, 2021 •

edited

Loading

azchohfi commented Feb 9, 2021 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.